Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add LaTeX support (v2) #3744

Closed
wants to merge 10 commits into from
Closed

Conversation

rk-for-zulip
Copy link
Contributor

Add LaTeX support. Fixes #2660.

Continuation and/or revision of #3679, which was closed due to overloading GitHub's UI with too many tangents. (Sibling, in some sense, of #3705.)

Factor `rsync` invocation into a function.

(This is not terribly useful yet, but will be once more rsync
invocations start happening.)
Add KaTeX (the version currently used by the server) as a dependency.
Pre-sync (and, if they ever exist, post-sync) scripts will be located
in a subdirectory of tools/, named `build-webview.d` (following the
modern Linux convention of placing configuration file-fragments or
auxiliary scripts in similarly named directories).

(No such scripts are included in this particular commit.)
Add a pre-build script to copy the KaTeX fonts and support files into
the staging directory for later bundling/packaging.

Add links to those files in the WebView HTML head.

* N.B.: Although the installation directions just say to include
  katex.[min.]js unconditionally, the KaTeX JavaScript isn't actually
  needed just to display existing KaTeX blocks -- only to render new
  ones.

(At this point, most inline equations and many block equations are
visible with their intended formatting.)
Wrap each block KaTeX equation in a small shim allowing it to be
scrolled horizontally.

Since the shim doesn't come with a visible scrollbar, add a visible
border in lieu thereof.
... and eliminate the legacy meaning of an unspecified second parameter.

This potentially involves some minor functional changes to
`handleInitialLoad`, in that the order of actions therewithin is
changed -- but if those actions don't commute, then we should probably
have been rewriting the source HTML before scrolling anyway.
KaTeX `.frac-line`s don't show up on some of our Android test devices
and emulators. (This seems to be a perennial problem for KaTeX. See
the commit-internal comments for references.)

Resolve this by forcing all such lines to be 1px wide, disregarding
the TeXbook spec.
Mixing permanent Git-controlled files and temporary build-"generated"
files in the same directory is... probably not a great idea.

Instead, use a dedicated external directory as a staging directory,
and treat `src/webview/static/` as just another source for files.
Using the `--delete-excluded` option everywhere causes the auxiliary
scripts to be noncommutative: in particular, the `00-base` script will
delete the preexisting `katex` subdirectory from the staging
directory, even when there was no need.

Remove it from the common invocation, and specify it explicitly only
for the separate staging-to-destination rsync.

----

N.B.: This leaves open the possibility that stale files will remain in
the staging directory, forgotten yet quietly relied upon. This will
need to be addressed -- but addressing it isn't expected to involve
significant changes to the auxiliary build scripts.
@rk-for-zulip
Copy link
Contributor Author

rk-for-zulip commented Feb 11, 2020

(Rebased and resolved merge conflicts. However, there are display issues on (at least) Android.)

@rk-for-zulip
Copy link
Contributor Author

The display issues mentioned above were resolved by a yarn install. Ready for review.

@gnprice
Copy link
Member

gnprice commented Feb 27, 2020

Marking as priority, following the issue.

@jedbrown
Copy link

I'd just like to comment that this is a really compelling feature for the many research groups that grumble daily about Slack lacking math and language-specific code. "That all works in Zulip" is a much stronger pitch than "it works with some clients". Thanks, @ray-kraesig, for implementing, and thanks in advance for the review, @gnprice.

@ghost
Copy link

ghost commented Mar 20, 2020

We've been waiting for this for a long time. Looking forward to it!

This was referenced Mar 27, 2020
@gnprice
Copy link
Member

gnprice commented Mar 27, 2020

Thanks @jedbrown and @pyrocto for those words of support!

I've just sent #4005, which incorporates several of @ray-kraesig's commits from this PR for their math-displaying goodness, and replaces the build-infrastructure pieces which I'd been concerned about the complexity of. I'm hoping we'll be able to merge and release this functionality soon.

Closing this PR in favor of code discussion there. I've also copied @jedbrown's and @pyrocto's comments over into the issue, #2660 .

@gnprice gnprice closed this Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LaTeX Support in mobile
3 participants